Skip to content

Conversation

@vladjdk
Copy link
Member

@vladjdk vladjdk commented Sep 19, 2025

evm/types served as a dumping ground for any objects and functions when they were seen to be miscellaneous. The existence of that folder is based on laziness and integrators expressed confusion between github.com/evm/types and github.com/evm/x/vm/types, as their name implies a similar function, when in fact, that was not always the case.

closes: #626

@vladjdk vladjdk changed the title Vlad/consolidate types refactor!: move types to respective folders and remove /types Sep 19, 2025
@vladjdk vladjdk marked this pull request as ready for review September 19, 2025 16:33
@vladjdk vladjdk requested review from a team as code owners September 19, 2025 16:33
@aljo242
Copy link
Contributor

aljo242 commented Sep 27, 2025

@vladjdk this lgtm probably best to ship after we finish the config work

Copy link
Member

@almk-dev almk-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (besides conflicts and CI complains, which I assume will be sorted in the future).
Not sure if the server1, server2 import namespace pattern is the prettiest, but I suppose it works.

Copy link
Contributor

@cloudgray cloudgray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks to this PR, each module has become much more modular!
I left a few comments about the import statements since some of them didn’t follow the conventions.

@vladjdk vladjdk changed the title refactor!: move types to respective folders and remove /types refactor!(types): move types to respective folders and remove /types Oct 9, 2025
@vladjdk vladjdk changed the title refactor!(types): move types to respective folders and remove /types refactor(types)!: move types to respective folders and remove /types Oct 9, 2025
Copy link
Contributor

@aljo242 aljo242 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two baby nits - otherwise lgtm

@vladjdk vladjdk added this pull request to the merge queue Oct 9, 2025
Merged via the queue into main with commit 61eedfc Oct 9, 2025
21 of 23 checks passed
@vladjdk vladjdk deleted the vlad/consolidate-types branch October 9, 2025 20:24
@cloudgray cloudgray mentioned this pull request Oct 10, 2025
3 tasks
zsystm pushed a commit to zsystm/evm that referenced this pull request Nov 2, 2025
cosmos#639)

* Move types to respective folders

* fix validation test

* use genesis type

* Revert "use genesis type"

This reverts commit 425ecfffadfa2e9e6ed069964eaa3034ae02e14a.

* lints

* lints - cont.

* changelog, reference fixes

* migration fix

* lint

* apply fixes

* some more 2 imports

* lints

* types2 no more

* Changelog

* add relevant import changes to migration guide

* proto changes

* fix comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove /evm/types and move functions to respective folders

6 participants